fix(tron): Enhance Tron direct transaction signing and address security issues#4737
Merged
sergei-boiko-trustwallet merged 13 commits intomasterfrom Apr 23, 2026
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens Tron transaction signing by shifting the “direct signing” flow to a validated raw_json input, introducing JSON deserialization back into the internal Tron protobuf transaction format, and adding a new error code for tx-hash mismatches.
Changes:
- Removed legacy direct signing via
txIdand madeSigningInput.raw_jsonthe preferred direct-signing input with txID/hash validation. - Added Tron JSON deserialization (
transactionFromJSON) plus extensive test coverage for raw JSON signing and validation failures. - Updated preimage/signing/compilation flows to return structured outputs (including tx compiler pre-signing output).
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/chains/Tron/TransactionCompilerTests.cpp | Splits out invalid raw JSON compile test case. |
| tests/chains/Tron/SignerTests.cpp | Reworks tests around raw JSON signing + adds tx-hash mismatch / unsupported cases. |
| tests/chains/Tron/DeserializationTests.cpp | New unit tests for JSON → internal transaction deserialization round-trips and invalid inputs. |
| swift/Tests/Blockchains/TronTests.swift | Removes txID-direct signing test; adds raw JSON signing test. |
| src/proto/Tron.proto | Removes legacy txId; documents raw_json behavior and expected errors. |
| src/proto/Common.proto | Adds Error_tx_hash_mismatch. |
| src/Tron/Signer.h | Adds raw JSON signing/compile/preimage APIs and structured preimage output type. |
| src/Tron/Signer.cpp | Implements raw JSON validation/signing/compilation and structured preimage output. |
| src/Tron/Entry.cpp | Updates preimage flow to use structured pre-signing output. |
| src/Tron/Deserialization.h / .cpp | Adds JSON deserialization into protocol::Transaction for supported contract types. |
| rust/tw_proto/src/impls.rs | Adds display string for the new signing error. |
| android/.../TestTronTransactionSigner.kt | Removes txId-direct signing test; adds raw JSON signing test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Binary size comparison➡️ aarch64-apple-ios: - 14.36 MB
+ 14.36 MB +1 KB➡️ aarch64-apple-ios-sim: 14.37 MB ➡️ aarch64-linux-android: - 18.80 MB
+ 18.81 MB +1 KB➡️ armv7-linux-androideabi: - 16.23 MB
+ 16.23 MB +1 KB➡️ wasm32-unknown-emscripten: - 13.72 MB
+ 13.72 MB +1 KB |
BSCSecChef
previously approved these changes
Apr 22, 2026
Collaborator
BSCSecChef
left a comment
BSCSecChef
left a comment
There was a problem hiding this comment.
Size check is added across and for the empty check in serialization - if crafted json is passed which has all empty values, but even if one element is passed, searlizeAsString will have non-empty bytes and not match he txID.
Changes look good
…elds and validations
nikhil-gupta-tw
approved these changes
Apr 22, 2026
Collaborator
|
LGTM |
BSCSecChef
approved these changes
Apr 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant improvements to the Tron transaction signing logic, focusing on robust handling, validation, and deserialization of raw JSON transactions. The changes remove legacy direct signing by txID, enforce strict checks on transaction hashes, and add comprehensive error reporting for malformed or tampered transactions. Additionally, the codebase is refactored to use a more structured and safer approach for handling raw JSON transactions across the signing, compiling, and preimage generation flows.
Key changes include:
Raw JSON Transaction Handling and Validation
Added new deserialization and validation logic for raw JSON transactions, ensuring that the provided
txIDmatches the hash of bothraw_data_hexand the serialized transaction. Detailed error codes and messages are returned for mismatches or unsupported cases.The
raw_jsonfield inSigningInputis now the preferred way to provide transactions for signing; the legacytxIdfield and related direct sign logic have been removed.Error Handling and Reporting
Introduced a new error code,
Error_tx_hash_mismatch, for cases where the transaction hash does not match the derived hash from the raw transaction data, improving diagnostics for tampered or malformed transactions.Improved error propagation and reporting throughout the signing and preimage generation flows, returning detailed messages and codes in the output proto.
Test and API Updates
txId.Internal Refactoring